api: Expose reference-vector helpers - #53
BenWestgate wants to merge 2 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated review (Claude), posted at the maintainer's request.
ACK 7311db8
- Pure renames plus
checksum_for_body_lengthfactored out of_from_partswith the same logic. I ran the api.md example; it parses as aSecret.__all__is 24. - These names become a supported v1 surface. That's the point of #49, but it's a commitment worth accepting on purpose.
- Sequencing: it conflicts with #42 and #57 (mechanical renames only).
- Nit: the new docstrings have a blank line after them, unlike the neighbouring functions.
|
Merge-order note from #42 review: #42 introduces |
|
Review follow-up: the blank-line docstring nit is valid but non-functional. I’m leaving the current one-commit API diff intact until #42 and #57 land because #53 already has mechanical rename conflicts with both; remove the extra blank lines in that single conflict-refresh commit rather than creating another pre-conflict churn commit. Human review order: #42 and #57 before #53. |
|
Second merge-order/API-boundary note from #13 review: after #13 lands, deduplicate the two ASCII-only case-fold implementations by adding one private |
|
Review-submission follow-up: ACK stands. The blank-line-after-docstring nit is style-only; handle it when this branch is refreshed after #13/#42 so the conflict resolution stays one mechanical API-boundary pass. That same refresh should (1) centralize private |
|
Release-gate sequencing note: keep this after #13, #42, and #57 because the current conflicts are mechanical renames. One concrete helper-boundary cleanup should be folded into this pass: #13 currently has equivalent ASCII-only lowercase helpers in |
|
Agent API-boundary review at current head |
Promote the module-level checksum and u5 conversion interfaces needed by reference-vector authors while keeping the package-level API narrow. Document and test the supported vector workflow and justify the remaining private cross-module couplings.\n\nValidation: 866 normal and 866 optimized tests; mypy; Ruff; production size budget.\n\nfixes #49
The newly supported chars_to_u5 interface must not case-fold Unicode into valid Bech32 symbols. Lower only ASCII characters so lookalikes such as the Kelvin sign remain invalid, matching the normalization boundary enforced elsewhere.
316ea11 to
cde312b
Compare
Closes #49.
Promote the module-level checksum specifications, u5 conversion helpers, and checksum-selection helpers required to construct and verify codex32 reference vectors without underscore-prefixed imports. These are supported at their owning modules; this PR does not add them to package-level
codex32.__all__.docs/developer/api.mddocuments the supported vector workflow and the remaining intentionally private cross-module couplings. Internal benchmarks may continue to use implementation-private names when they are explicitly testing internals.The supported
chars_to_u5()API lowercases ASCII only. Unicode lookalikes such as Kelvin signKstay invalid instead of being case-folded into Bech32k. The ASCII-only helper now lives once inbech32.pyand is shared by the CLI input path.Review stack
Base: #52 (
codex/5-release-qualification). Current head:cde312b.This is the planned one-time refresh after the overlapping restore/foundation/security/release stack settled. Mechanical conflict resolution preserves the reviewed mixed-case correction state, the Core-native process boundary, the 83-character BIP173 HRP rule, and the supported module-level vector API. Package-level
codex32.__all__remains 23 names.The diff remains two logical review units:
d6679cc— the human-authored vector-API publication, replayed on the settled base;cde312b— the Unicode-lookalike follow-up plus the mechanical ASCII-lowercase centralization required by the settled CLI/Core code.The second commit remains Codex-authored and must be rewritten/squashed under the responsible human author before merge, per the repository authorship policy. The PR therefore remains draft until that human step is performed.
Validation
On current head
cde312b:python -O: 952 passed (plus the expected pytest optimized-mode warning);python -O;__all__: 23 names;git diff --check: clean.GitHub CI is re-running for the refreshed head. All previously filed inline review threads were resolved before the refresh; re-review the refreshed diff after CI settles.
Disclosure: AI tools assisted with the mechanical restack and conflict analysis. Human responsibility is required for final authorship and integration.